Skip to content

feat(iswf): Removes retry decorator from workflow_engine tasks#114937

Merged
GabeVillalobos merged 2 commits intomasterfrom
gv/removes-retry-decorators-from-wfe-tasks
May 6, 2026
Merged

feat(iswf): Removes retry decorator from workflow_engine tasks#114937
GabeVillalobos merged 2 commits intomasterfrom
gv/removes-retry-decorators-from-wfe-tasks

Conversation

@GabeVillalobos
Copy link
Copy Markdown
Member

@GabeVillalobos GabeVillalobos commented May 5, 2026

Removes usages of the @retry decorator from workflow engine tasks, translating them into their taskbroker equivalent.

Relies on #114936 to be merged first.

@GabeVillalobos GabeVillalobos requested review from a team as code owners May 5, 2026 23:52
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label May 5, 2026
Comment thread src/sentry/workflow_engine/tasks/delayed_workflows.py Outdated
Comment on lines +79 to +89
times=3,
delay=5,
on=(Exception, ProcessingDeadlineExceeded),
ignore=(
Action.DoesNotExist,
Group.DoesNotExist,
Project.DoesNotExist,
ProjectNotActiveError,
Workflow.DoesNotExist,
),
),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The trigger_action task will now fail loudly after exhausting retries instead of silently completing, due to a missing times_exceeded parameter.
Severity: MEDIUM

Suggested Fix

To restore the previous behavior of silently discarding the task after all retries are exhausted, add times_exceeded=LastAction.Discard to the Retry object in the trigger_action task.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/workflow_engine/tasks/actions.py#L78-L89

Potential issue: The `trigger_action` task's behavior upon exhausting its retries has
changed. Previously, it used `raise_on_no_retries=False` to silently complete without
error if all retries failed. The new implementation using the `Retry` object lacks an
equivalent configuration, such as `times_exceeded=LastAction.Discard`. As a result,
after exhausting its retries, the task will now raise an exception and be marked as a
failure, which is a change from its previous behavior.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment thread src/sentry/workflow_engine/tasks/actions.py
@GabeVillalobos GabeVillalobos requested a review from markstory May 6, 2026 00:00
Comment thread src/sentry/workflow_engine/tasks/delayed_workflows.py Outdated
Project.DoesNotExist,
ProjectNotActiveError,
Workflow.DoesNotExist,
),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lost Sentry capture for ignore_and_capture exceptions

Medium Severity

The old @retry decorator distinguished between ignore_and_capture=(Action.DoesNotExist, Group.DoesNotExist) (no retry, task succeeds, but explicitly captured to Sentry at "info" level) and ignore=(Project.DoesNotExist, ...) (no retry, task succeeds, no Sentry report). The new code places all five exceptions into Retry.ignore, losing the intentional sentry_sdk.capture_exception(level="info") call for Action.DoesNotExist and Group.DoesNotExist. The deliberate exclusion of these two from silenced_exceptions suggests the author wanted them reported, but if Retry.ignore silently consumes the exception, it never reaches the reporting layer.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f649020. Configure here.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 33269fc. Configure here.

ProjectNotActiveError,
Workflow.DoesNotExist,
),
),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing times_exceeded=LastAction.Discard for exhausted retries

Medium Severity

The old @retry decorator for trigger_action had raise_on_no_retries=False, which meant that when all retries were exhausted, the task would complete successfully instead of failing. The new Retry configuration doesn't include the equivalent times_exceeded=LastAction.Discard parameter (which is used elsewhere in the codebase, e.g., scheduled.py). This means the task will now fail with an unhandled exception after retries are exhausted, instead of silently succeeding as before.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 33269fc. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base automatically changed from gv/add-new-retry-decorator-options to master May 6, 2026 16:46
@GabeVillalobos GabeVillalobos requested review from a team as code owners May 6, 2026 16:46
@GabeVillalobos GabeVillalobos force-pushed the gv/removes-retry-decorators-from-wfe-tasks branch from 33269fc to 1b4df89 Compare May 6, 2026 16:47
@armenzg armenzg removed the request for review from a team May 6, 2026 17:47
Copy link
Copy Markdown
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 Great to see this happening.

@GabeVillalobos GabeVillalobos merged commit 4ff3660 into master May 6, 2026
63 checks passed
@GabeVillalobos GabeVillalobos deleted the gv/removes-retry-decorators-from-wfe-tasks branch May 6, 2026 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants